Skip to content

feat: treat void as null for null coalescing operator #2494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

feat: treat void as null for null coalescing operator #2494

wants to merge 1 commit into from

Conversation

@kesselb
Copy link

@kesselb kesselb commented Jun 28, 2023

Hi 👋

WordPress often uses type definitions like array|void (e.g. https://github.com/WordPress/wordpress-develop/blob/68be569925ad2a88c97e1308b8a67a57bf17a14f/src/wp-includes/post.php#L2627).

$custom_keys = get_post_custom_keys( $post_id ) ?? [];

Shows: Expression on left side of ?? is not nullable.

@phpstan-bot
Copy link
Collaborator

phpstan-bot commented Jun 28, 2023

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@kesselb kesselb force-pushed the void-nullcoalesce branch from b885d99 to 28888d3 Compare June 28, 2023 19:40
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jun 28, 2023

Using void in a union is completely invalid and when used in a native type even crashes PHP: https://3v4l.org/7iULB

Instead of array|void instead array|null should be used.

That said, I'd accept a PR that changes array|void into array|null as a result of FuncCall/StaticCall/MethodCall. So the change needs to happen in MutatingScope::resolveType. It's not enough to change a single rule.

@kesselb
feat: treat void as null for null coalescing operator
cdaa291 
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb force-pushed the void-nullcoalesce branch from 28888d3 to cdaa291 Compare June 28, 2023 19:52
@kesselb
Copy link
Author

kesselb commented Jun 28, 2023

Thanks for your prompt review 👍

I will take a look.

@herndlm
Copy link
Contributor

herndlm commented Jun 28, 2023 •
edited
Loading

Looks like you are aware of it already @kesselb , but just for the sake of completeness and issue linking - we discussed this also in szepeviktor/phpstan-wordpress#176, and yeah, WordPress types are wrong there IMO

Oh and phpstan/phpstan#9185 too

@staabm staabm mentioned this pull request Dec 11, 2023
Merged
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Dec 11, 2023

Superseded by #2778

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kesselb @phpstan-bot @ondrejmirtes @herndlm